gh-138004: fix threadmodule ascii and make thread naming test more lenient #138017

jadonduff · 2025-08-21T06:16:01Z

Fallback to ASCII in _thread.set_name() when pthread_setname_np() rejects UTF-8

Summary

Issue: test_set_name fails on OpenIndiana (and Solaris descendants).
It seems that pthread_setname_np() only accepts ASCII-only names there:

>>> import _thread
>>> _thread.set_name('€')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 22] Invalid argument

Fix

1. `_threadmodule.c`

Attempt to encode the thread name in UTF-8 and call pthread_setname_np() as usual.
If the call fails with EINVAL, fall back to ASCII encoding using "replace" (non-ASCII → ?).
On macOS and other platforms that already accept UTF-8, the original UTF-8 succeeds and the fallback is never taken. ASCII names continue to work unchanged.

int rc;

// Fallback: If EINVAL, try ASCII encoding with "replace"
if (rc == EINVAL) {
    name_encoded = PyUnicode_AsEncodedString(name_obj, "ascii", "replace");
    if (name_encoded == NULL) {
        return NULL;
    }
#ifdef _PYTHREAD_NAME_MAXLEN
    if (PyBytes_GET_SIZE(name_encoded) > _PYTHREAD_NAME_MAXLEN) {
        PyObject *truncated;
        truncated = PyBytes_FromStringAndSize(PyBytes_AS_STRING(name_encoded),
                                              _PYTHREAD_NAME_MAXLEN);
        if (truncated == NULL) {
            Py_DECREF(name_encoded);
            return NULL;
        }
        Py_SETREF(name_encoded, truncated);
    }
#endif
    name = PyBytes_AS_STRING(name_encoded);
#ifdef __APPLE__
    rc = pthread_setname_np(name);
#elif defined(__NetBSD__)
    thread = pthread_self();
    rc = pthread_setname_np(thread, "%s", (void *)name);
#elif defined(HAVE_PTHREAD_SETNAME_NP)
    thread = pthread_self();
    rc = pthread_setname_np(thread, name);
#else /* defined(HAVE_PTHREAD_SET_NAME_NP) */
    thread = pthread_self();
    rc = 0;
    pthread_set_name_np(thread, name);
#endif
    Py_DECREF(name_encoded);
}

Explanation:

Keeps current behavior everywhere UTF-8 is supported.
Provides a fallback for platforms that only accept ASCII (e.g. OpenIndiana).
Avoids raising OSError(22) for non-ASCII names.

2. `test_thread.py`

Updated test_set_name to be lenient on platforms that reject non-ASCII names.
If OSError(22) occurs and the attempted name contained non-ASCII, the test is skipped instead of failing.

try:
    thread.start()
    thread.join()
    self.assertEqual(work_name, expected,
                     f"{len(work_name)=} and {len(expected)=}")
except OSError as exc:
    # Accept EINVAL (22) for non-ASCII names on platforms that do not support them
    if getattr(exc, 'errno', None) == 22 and any(ord(c) > 127 for c in name):
        self.skipTest(f"Platform does not support non-ASCII thread names: {exc}")
    else:
        raise

Explanation:

On macOS/Linux: runs as before, asserts the name round-trips correctly.
On OpenIndiana: skips the test if non-ASCII is not supported.

Verification

Ran locally on macOS (UTF-8 names continue to work as before).
Did not verify on OpenIndiana (cannot reproduce locally). Fix based on reports.
All tests passed on macOS ARM.

Issue: test_set_name fails on OpenIndiana (and Solaris?) #138004

python-cla-bot · 2025-08-21T06:16:05Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2025-08-21T06:16:06Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

serhiy-storchaka · 2025-08-21T08:58:16Z

Modules/_threadmodule.c

+        name_encoded = PyUnicode_AsEncodedString(name_obj, "ascii", "replace");
+        if (name_encoded == NULL) {
+            return NULL;
+        }
+#ifdef _PYTHREAD_NAME_MAXLEN
+        if (PyBytes_GET_SIZE(name_encoded) > _PYTHREAD_NAME_MAXLEN) {
+            PyObject *truncated;
+            truncated = PyBytes_FromStringAndSize(PyBytes_AS_STRING(name_encoded),
+                                                  _PYTHREAD_NAME_MAXLEN);
+            if (truncated == NULL) {
+                Py_DECREF(name_encoded);
+                return NULL;
+            }
+            Py_SETREF(name_encoded, truncated);
+        }
+#endif
+        name = PyBytes_AS_STRING(name_encoded);
+#ifdef __APPLE__
+        rc = pthread_setname_np(name);
+#elif defined(__NetBSD__)
+        thread = pthread_self();
+        rc = pthread_setname_np(thread, "%s", (void *)name);
+#elif defined(HAVE_PTHREAD_SETNAME_NP)
+        thread = pthread_self();
+        rc = pthread_setname_np(thread, name);
+#else /* defined(HAVE_PTHREAD_SET_NAME_NP) */
+        thread = pthread_self();
+        rc = 0;
+        pthread_set_name_np(thread, name);
+#endif
+        Py_DECREF(name_encoded);


Do not duplicate such complex code. Move it to function and reuse. Or use a loop (but this may be more complicated).

I moved to a function and reused. Despite most tests passing, I am receiving the following for the "Check if generated files are up to date" test:

/usr/bin/ld: Modules/_threadmodule.o: in function `_thread_set_name_impl': /home/runner/work/cpython/cpython/./Modules/_threadmodule.c:2617:(.text+0x2ed5): undefined reference to `_set_thread_name' /usr/bin/ld: /home/runner/work/cpython/cpython/./Modules/_threadmodule.c:2637:(.text+0x2f99): undefined reference to `_set_thread_name' collect2: error: ld returned 1 exit status make: *** [Makefile:1927: Programs/_freeze_module] Error 1 Error: Process completed with exit code 2.

Please let me know if you see the issue. It appeared to be working before changing to function.

serhiy-storchaka · 2025-08-21T08:59:52Z

Lib/test/test_threading.py

+                    thread.join()
+                    self.assertEqual(work_name, expected,
+                                     f"{len(work_name)=} and {len(expected)=}")
+                except OSError as exc:


Is OSError even raised here? The test failure was different -- that work_name was unexpectedly empty.

Added:

if any(ord(c) > 127 for c in name) and (not work_name or work_name == ""): self.skipTest(f"Platform does not support non-ASCII thread names: got empty name for {name!r}") self.assertEqual(work_name, expected, f"{len(work_name)=} and {len(expected)=}")

Kept OSError fallback because it was shown in original issue, but can remove if needed.

>>> import _thread >>> _thread.set_name('€') Traceback (most recent call last): File "<python-input-1>", line 1, in <module> _thread.set_name('€') ~~~~~~~~~~~~~~~~^^^^^ OSError: [Errno 22] Invalid argument

It was shown that _thread.set_name() fails, but that OSError is not leaked from the Thread code.

Misc/NEWS.d/next/Core_and_Builtins/2025-08-21-06-31-42.gh-issue-138004.FH2Hre.rst

bedevere-app · 2025-08-21T09:06:39Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

jadonduff · 2025-08-22T05:03:03Z

Please see comments regarding the requested changes. OS tests passed, but a function not found error occurred in "Tests / Check if generated files are up to date (pull_request)", which I am having trouble fixing. Apologies - this is my first attempt at contributing to Python.

jadonduff · 2025-08-22T05:03:19Z

I have made the requested changes; please review again

bedevere-app · 2025-08-22T05:03:24Z

Thanks for making the requested changes!

@serhiy-storchaka: please review the changes made to this pull request.

Modules/_threadmodule.c

ZeroIntensity · 2025-08-22T11:20:24Z

Modules/_threadmodule.c

+#ifdef _PYTHREAD_NAME_MAXLEN
+        if (PyBytes_GET_SIZE(name_encoded) > _PYTHREAD_NAME_MAXLEN) {
+            PyObject *truncated = PyBytes_FromStringAndSize(PyBytes_AS_STRING(name_encoded), _PYTHREAD_NAME_MAXLEN);
+            if (truncated == NULL) {
+                Py_DECREF(name_encoded);
+                return NULL;
+            }
+            Py_SETREF(name_encoded, truncated);
+        }
+#endif


Again, please try to avoid duplicating code. This should be factored out into its own function. Here's an outline:

static PyObject * get_truncated(PyObject *name_encoded /* stolen */) { #ifdef _PYTHREAD_NAME_MAXLEN if (PyBytes_GET_SIZE(name_encoded) > _PYTHREAD_NAME_MAXLEN) { PyObject *truncated = PyBytes_FromStringAndSize(PyBytes_AS_STRING(name_encoded), _PYTHREAD_NAME_MAXLEN); if (truncated == NULL) { Py_DECREF(name_encoded); return NULL; } Py_SETREF(name_encoded, truncated); } #endif return name_encoded; }

Or simply include the encoding and truncating code in _set_thread_name(). BTW, there is no need to use underscored names here.

Created function encode_thread_name and used to replace duplicated code

ZeroIntensity · 2025-08-22T11:21:35Z

Lib/test/test_threading.py

@@ -2241,6 +2241,7 @@ def __init__(self, a, *, b) -> None:

        with warnings.catch_warnings(record=True) as warnings_log:
            CustomRLock(1, b=2)
+


Stray newline change:

Suggested change

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

jadonduff · 2025-08-22T18:50:31Z

Pulled _threadmodule.c from main and re-wrote functions. Added two functions - set_native_thread_name and encode_thread_name (using ZeroIntensity's outline). Also removed OSError exception handing in test_threading.py (serhiy-storchaka)

serhiy-storchaka · 2025-08-22T19:44:38Z

Modules/_threadmodule.c

+            name_encoded = encode_thread_name(name_obj, "ascii");
+            if (name_encoded == NULL) {
+                return NULL;
+            }
+            name = PyBytes_AS_STRING(name_encoded);
+            rc = set_native_thread_name(name);


You can merge all this (and the following Py_DECREF(name_encoded)) in a single function.

serhiy-storchaka · 2025-08-22T20:03:19Z

Lib/test/test_threading.py

@@ -2360,6 +2360,9 @@ def work():
                thread = threading.Thread(target=work, name=name)
                thread.start()
                thread.join()
+                # If the name is non-ASCII and the result is empty, skip (platform limitation)
+                if any(ord(c) > 127 for c in name) and (not work_name or work_name == ""):


You can use the isascii() method. And why not work_name or work_name == ""?

Used isascii() method, and changed to:

# If the name is non-ASCII and the result is empty, skip (platform limitation) if not name.isascii() and not work_name: self.skipTest(f"Platform does not support non-ASCII thread names: got empty name for {name!r}") self.assertEqual(work_name, expected, f"{len(work_name)=} and {len(expected)=}")

serhiy-storchaka · 2025-08-22T20:06:06Z

Modules/_threadmodule.c

-#ifdef __sun
-    // Solaris always uses UTF-8
-    const char *encoding = "utf-8";
-#else
-    // Encode the thread name to the filesystem encoding using the "replace"
-    // error handler


Why remove all this?

moved to encode_thread_name

serhiy-storchaka · 2025-08-22T20:06:45Z

Modules/_threadmodule.c

@@ -2894,4 +2924,4 @@ PyMODINIT_FUNC
 PyInit__thread(void)
 {
    return PyModuleDef_Init(&thread_module);
-}
+}


Unrelated change (newline?).

May have accidentally added a newline in a previous commit. It now matches the current _threadmodule.c in the main branch.

Please revert this change.

Modules/_threadmodule.c

picnixz · 2025-08-22T20:45:28Z

Modules/_threadmodule.c

    Py_RETURN_NONE;
 #else
    // Windows implementation
    assert(pSetThreadDescription != NULL);
-


Those blank lines could be kept as this part of the code is not touched.

Modules/_threadmodule.c

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

serhiy-storchaka · 2025-08-23T07:43:13Z

Lib/test/test_threading.py

@@ -2360,6 +2361,9 @@ def work():
                thread = threading.Thread(target=work, name=name)
                thread.start()
                thread.join()
+                # If the name is non-ASCII and the result is empty, skip (platform limitation)


Do not repeat the code in a comment.

serhiy-storchaka · 2025-08-23T07:44:20Z

Modules/_threadmodule.c

@@ -2894,4 +2924,4 @@ PyMODINIT_FUNC
 PyInit__thread(void)
 {
    return PyModuleDef_Init(&thread_module);
-}
+}


Please revert this change.

serhiy-storchaka · 2025-08-23T07:46:43Z

Modules/_threadmodule.c

    if (rc) {
-        errno = rc;
+        int err = rc;


Why it was needed to introduce the err variable?

serhiy-storchaka · 2025-08-23T07:49:33Z

Modules/_threadmodule.c

+        Py_DECREF(name_encoded);
+        return truncated;


serhiy-storchaka · 2025-08-23T07:53:01Z

Modules/_threadmodule.c

+    PyObject *name_encoded = encode_thread_name(name_obj, encoding);
+    if (name_encoded == NULL) {
+        return -1; // error, exception set
+    }
+    const char *name = PyBytes_AS_STRING(name_encoded);
+    int rc = set_native_thread_name(name);
+    Py_DECREF(name_encoded);
+    return rc;


You can now inline set_native_thread_name() and encode_thread_name() as it was in the original code.

serhiy-storchaka · 2025-08-23T07:53:44Z

Modules/_threadmodule.c

+{
+    PyObject *name_encoded = encode_thread_name(name_obj, encoding);
+    if (name_encoded == NULL) {
+        return -1; // error, exception set


What happens at the caller place when set_thread_name_with_encoding() returns -1?

fix threadmodule ascii and make test more lenient

d97417d

bedevere-app bot added the awaiting review label Aug 21, 2025

bedevere-app bot mentioned this pull request Aug 21, 2025

test_set_name fails on OpenIndiana (and Solaris?) #138004

Open

📜🤖 Added by blurb_it.

1db08a7

serhiy-storchaka requested changes Aug 21, 2025

View reviewed changes

bedevere-app bot removed the awaiting review label Aug 21, 2025

bedevere-app bot added the awaiting changes label Aug 21, 2025

jadonduff added 10 commits August 21, 2025 20:42

implement requested changes

0606968

reformat and remove test news

b556774

patch for windows & non-posix compliant platforms

38a75d3

patch

31731b1

Check if generated files are up to date patch

83fe205

test

aadf7f3

test2

612a0a4

test3

1fa51f8

attempt fix

d24d0bb

Merge branch 'main' into thread_name_fix

95d289f

bedevere-app bot added awaiting change review and removed awaiting changes labels Aug 22, 2025

bedevere-app bot requested a review from serhiy-storchaka August 22, 2025 05:03

ZeroIntensity reviewed Aug 22, 2025

View reviewed changes

jadonduff and others added 3 commits August 22, 2025 13:28

test

d7a47bf

fix stray newline

1970a00

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

Merge branch 'main' into thread_name_fix

6395323

jadonduff requested a review from ZeroIntensity August 22, 2025 19:53

serhiy-storchaka reviewed Aug 22, 2025

View reviewed changes

picnixz reviewed Aug 22, 2025

View reviewed changes

jadonduff and others added 3 commits August 22, 2025 16:58

Apply suggestions from code review

241e097

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

fixes

66c058b

apply fixes (built)

f817412

serhiy-storchaka reviewed Aug 23, 2025

View reviewed changes

		@@ -2241,6 +2241,7 @@ def __init__(self, a, *, b) -> None:

		with warnings.catch_warnings(record=True) as warnings_log:
		CustomRLock(1, b=2)

Uh oh!

gh-138004: fix threadmodule ascii and make thread naming test more lenient #138017

Are you sure you want to change the base?

gh-138004: fix threadmodule ascii and make thread naming test more lenient #138017

Conversation

jadonduff commented Aug 21, 2025 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fallback to ASCII in _thread.set_name() when pthread_setname_np() rejects UTF-8

Summary

Fix

1. _threadmodule.c

2. test_thread.py

Verification

Uh oh!

python-cla-bot bot commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app bot commented Aug 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jadonduff Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jadonduff Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bedevere-app bot commented Aug 21, 2025

Uh oh!

jadonduff commented Aug 22, 2025

Uh oh!

jadonduff commented Aug 22, 2025

Uh oh!

bedevere-app bot commented Aug 22, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jadonduff commented Aug 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jadonduff commented Aug 21, 2025 •

edited by bedevere-app bot

Loading

1. `_threadmodule.c`

2. `test_thread.py`

python-cla-bot bot commented Aug 21, 2025 •

edited

Loading

jadonduff Aug 22, 2025 •

edited

Loading

jadonduff Aug 22, 2025 •

edited

Loading